Fix Makie recipe to support 3D plots with automatic LScene detection #1079
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds `preferred_axis_type` methods for AbstractTimeseriesSolution, DEIntegrator, and AbstractEnsembleSolution that automatically detect when 3D plotting is requested (idxs=(1,2,3)) and return LScene as the preferred axis type instead of the default Axis. This fixes issue #947 where 3D plots failed because Makie defaulted to 2D Axis instead of using LScene for 3D visualizations. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| # Define preferred axis type based on the idxs parameter | ||
| function Makie.preferred_axis_type(plot::Plot{<:Tuple{<:SciMLBase.AbstractTimeseriesSolution}}) | ||
| if haskey(plot, :idxs) || haskey(plot, :vars) | ||
| idxs = haskey(plot, :idxs) ? plot[:idxs][] : (haskey(plot, :vars) ? plot[:vars][] : nothing) |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
Suggested change
| idxs = haskey(plot, :idxs) ? plot[:idxs][] : (haskey(plot, :vars) ? plot[:vars][] : nothing) | |
| idxs = haskey(plot, :idxs) ? plot[:idxs][] : | |
| (haskey(plot, :vars) ? plot[:vars][] : nothing) |
| if idxs isa Tuple && length(idxs) == 3 | ||
| return Makie.LScene | ||
| elseif idxs isa AbstractArray && length(idxs) == 3 | ||
| return Makie.LScene |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
Suggested change
| return Makie.LScene | |
| return Makie.LScene |
| # Define preferred axis type for integrator plots | ||
| function Makie.preferred_axis_type(plot::Plot{<:Tuple{<:SciMLBase.DEIntegrator}}) | ||
| if haskey(plot, :idxs) || haskey(plot, :vars) | ||
| idxs = haskey(plot, :idxs) ? plot[:idxs][] : (haskey(plot, :vars) ? plot[:vars][] : nothing) |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
Suggested change
| idxs = haskey(plot, :idxs) ? plot[:idxs][] : (haskey(plot, :vars) ? plot[:vars][] : nothing) | |
| idxs = haskey(plot, :idxs) ? plot[:idxs][] : | |
| (haskey(plot, :vars) ? plot[:vars][] : nothing) |
| if idxs isa Tuple && length(idxs) == 3 | ||
| return Makie.LScene | ||
| elseif idxs isa AbstractArray && length(idxs) == 3 | ||
| return Makie.LScene |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
Suggested change
| return Makie.LScene | |
| return Makie.LScene |
| # Define preferred axis type for ensemble plots | ||
| function Makie.preferred_axis_type(plot::Plot{<:Tuple{<:SciMLBase.AbstractEnsembleSolution}}) | ||
| if haskey(plot, :idxs) || haskey(plot, :vars) | ||
| idxs = haskey(plot, :idxs) ? plot[:idxs][] : (haskey(plot, :vars) ? plot[:vars][] : nothing) |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
Suggested change
| idxs = haskey(plot, :idxs) ? plot[:idxs][] : (haskey(plot, :vars) ? plot[:vars][] : nothing) | |
| idxs = haskey(plot, :idxs) ? plot[:idxs][] : | |
| (haskey(plot, :vars) ? plot[:vars][] : nothing) |
| if idxs isa Tuple && length(idxs) == 3 | ||
| return Makie.LScene | ||
| elseif idxs isa AbstractArray && length(idxs) == 3 | ||
| return Makie.LScene |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
Suggested change
| return Makie.LScene | |
| return Makie.LScene |
|
@asinghvi17 can you look at this one? |
asinghvi17
suggested changes
Jul 28, 2025
| end | ||
|
|
||
| # Define preferred axis type based on the idxs parameter | ||
| function Makie.preferred_axis_type(plot::Plot{<:Tuple{<:SciMLBase.AbstractTimeseriesSolution}}) |
There was a problem hiding this comment.
This dispatch doesn't actually do anything. I don't think you can actually hook into this in this way in Makie v0.24. It might need a small patch, I'm PRing that to Makie now
|
Gentle bump to this issue. In our 3D plot demo, we found that only CairoMakie is able to plot the 3D trajectory. When we switched to GLMakie, we got the following error: ERROR: Cannot register computation: The outputs already have a parent compute edge using a different callback function.CairoMakie v0.15.6
GLMakie v0.13.6
TestParticle v0.14.2The proposed workaround is fine for GLMakie plot(sol, idxs=(1,2,3); axis = (; type = LScene)) |
|
I don't think this PR is it. It sounds like @asinghvi17 had an idea of how to do it right though. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
preferred_axis_typemethods for SciML solution typesThe Problem
When plotting a 3D system using the Makie recipe with
idxs = (1, 2, 3), the plot fails because Makie defaults to a 2DAxisinstead of usingLScenefor 3D visualizations. Users had to manually specifyaxis = (; type = LScene)as a workaround.The Solution
Added
preferred_axis_typemethods for:AbstractTimeseriesSolutionDEIntegratorAbstractEnsembleSolutionThese methods check if
idxs(or the deprecatedvars) parameter contains exactly 3 indices, and if so, returnMakie.LSceneas the preferred axis type. Otherwise, they default toMakie.Axis.Test plan
🤖 Generated with Claude Code